Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CH59x bringup and blinky #534

Merged
merged 6 commits into from
Mar 12, 2025
Merged

CH59x bringup and blinky #534

merged 6 commits into from
Mar 12, 2025

Conversation

biemster
Copy link
Contributor

CH592/1 bringup and blinky. This is the start of CH59x support, and will conclude in merging the https://github.com/biemster/ch592_noblob_advertiser functionality in here.

One major issue is that the CH59x SDK is not based on structs like ch32, but just pointer #defines. Also I could not find anything resembling RCC in those #defines, so this might need some refactoring on both sides.

@biemster biemster marked this pull request as draft February 24, 2025 13:54
@cnlohr
Copy link
Owner

cnlohr commented Feb 25, 2025

Thanks for doing this work, @biemster , I've added comments, it looks like you're really close, but it needs a little bit of cleanup.

@biemster
Copy link
Contributor Author

yeah I basically just copied in the bare necessities, so the reviewer can see what all needs to go in properly. Cleanup is definitely necessary, and I might add a small bit of extra stuff on the gpio side of things to handle GPIOB and interrupts as well. Where did you add your comments?

@cnlohr
Copy link
Owner

cnlohr commented Feb 25, 2025

Sorry, I forgot to commit the review.

@biemster
Copy link
Contributor Author

biemster commented Feb 26, 2025

@cnlohr I went through all the comments, I was able to accommodate the changes to sys_safe_access_ and GPIOx_ModeCfg I think. I couldn't struct the flash registers since they seem very different from ch32. The overbuilt part in ch32fun.c is completely gone, as everything lives in #defines now. Could you review again?

edit: those last two force-pushes were to add funDigitalRead, just three lines in ch32fun.h

@cnlohr
Copy link
Owner

cnlohr commented Feb 26, 2025

@biemster you will need to make at least one example, even if it's just blink but, I would really like it if you also had a debugprintf example as well. It feels like minimum viable product is blink and printf where both use the ch32fun functions / structure. I.e. blink.c should be an exact copy of blink.c from example but maybe with a different IO pin.

@biemster
Copy link
Contributor Author

biemster commented Feb 26, 2025

Did I forget to commit the blink? I definitely made one, let me check. I was also thinking of a printf indeed, since writing to flash does not want to play ball at all currently. I'll focus on that coming days, and add it to the PR.

edit: blink is there :) it does not have funGpioInitAll since there is not really a similar thing on ch59x, and it uses the pin definitions from ch59xhw.h. I'm not sure how to translate the ch32 ones to ch5x, they don't have a MHz definition in them, just current like 5mA and 20mA..

@cnlohr
Copy link
Owner

cnlohr commented Feb 26, 2025

You can "overload" them to the MHz functions, but also have your own that define 0, 1, 2, 3, for the mA modes.

Like you should allow users to say 10MHz PP, and just be like 🤷 I guess I'll just treat it as the 2nd highest current.

@cnlohr
Copy link
Owner

cnlohr commented Feb 26, 2025

I do see blink.c Sorry I missed it first time around, but still need to think through printf.

@biemster
Copy link
Contributor Author

biemster commented Feb 27, 2025

I do see blink.c Sorry I missed it first time around, but still need to think through printf.

I noticed the printf going through a debug interface on the ch32 chips, but I did not find anything like that on ch59x. Could I just do that over uart?
edit: o wait I just noticed the comments on printf in ch32fun.h, I'll use an uart

@biemster
Copy link
Contributor Author

You can "overload" them to the MHz functions, but also have your own that define 0, 1, 2, 3, for the mA modes.

Like you should allow users to say 10MHz PP, and just be like 🤷 I guess I'll just treat it as the 2nd highest current.

Should there be a warning emitted when a pin definition is way off? For example this:

typedef enum
{
	...
	GPIO_CFGLR_IN_PUPD = 8,

ch59x has GPIO_ModeIN_PU and GPIO_ModeIN_PD, which are very different as far as I understand. Also ch59x does not seem to have open-drain available, only push-pull so a user might really need open-drain and then I give then push-pull (I don't want to be on the debugging side of that if it causes issues with the user's design :D )

@cnlohr
Copy link
Owner

cnlohr commented Feb 27, 2025

I do see blink.c Sorry I missed it first time around, but still need to think through printf.

I noticed the printf going through a debug interface on the ch32 chips, but I did not find anything like that on ch59x. Could I just do that over uart? edit: o wait I just noticed the comments on printf in ch32fun.h, I'll use an uart

Please if at all possible make default to go over debug interface. My comments were just that you may be able to use a two-word debug output. SWIO debugging is soooo much more convenient than UART debugging.

@cnlohr
Copy link
Owner

cnlohr commented Feb 27, 2025

You can "overload" them to the MHz functions, but also have your own that define 0, 1, 2, 3, for the mA modes.
Like you should allow users to say 10MHz PP, and just be like 🤷 I guess I'll just treat it as the 2nd highest current.

Should there be a warning emitted when a pin definition is way off? For example this:

typedef enum
{
	...
	GPIO_CFGLR_IN_PUPD = 8,

ch59x has GPIO_ModeIN_PU and GPIO_ModeIN_PD, which are very different as far as I understand. Also ch59x does not seem to have open-drain available, only push-pull so a user might really need open-drain and then I give then push-pull (I don't want to be on the debugging side of that if it causes issues with the user's design :D )

Do you mind sharing notes with @dwillmore here? See what his opinions are because the 00x drive mechanism is also different than the ch32vxxx line.

image

I agree that GPIO_CFGLR_IN_PUPD is not possible, so please leave that definition out.

@cnlohr
Copy link
Owner

cnlohr commented Feb 27, 2025

About the debug interface, it looks like it should JustWork. The CH592 uses a QingKeV4, which has both DMDEBUG0 and DMDEBUG1 interfaces.

@biemster
Copy link
Contributor Author

About the debug interface, it looks like it should JustWork. The CH592 uses a QingKeV4, which has both DMDEBUG0 and DMDEBUG1 interfaces.

really? that's awesome! i have zero experience with such, but if it's there I'll get it to work

@biemster
Copy link
Contributor Author

biemster commented Mar 1, 2025

About the debug interface, it looks like it should JustWork. The CH592 uses a QingKeV4, which has both DMDEBUG0 and DMDEBUG1 interfaces.

This is going to be a guessing game, as the datasheet does not mention the debug interfaces or a SWIO pin at all! Same for ch32x305 though, which does have a debugprintfdemo, so I'm hoping it's on a similar pin

@biemster
Copy link
Contributor Author

biemster commented Mar 1, 2025

About the debug interface, it looks like it should JustWork. The CH592 uses a QingKeV4, which has both DMDEBUG0 and DMDEBUG1 interfaces.

This is going to be a guessing game, as the datasheet does not mention the debug interfaces or a SWIO pin at all! Same for ch32x305 though, which does have a debugprintfdemo, so I'm hoping it's on a similar pin

@cnlohr how do I test this? My linkE (and linkW) are still on the slow boat, and my ESP32's are all none s2 ones 😭
edit: I just found 3 s2mini's on the bottom of my parts bin, I'm back on the horse!

@cnlohr
Copy link
Owner

cnlohr commented Mar 1, 2025

About the debug interface, it looks like it should JustWork. The CH592 uses a QingKeV4, which has both DMDEBUG0 and DMDEBUG1 interfaces.

This is going to be a guessing game, as the datasheet does not mention the debug interfaces or a SWIO pin at all! Same for ch32x305 though, which does have a debugprintfdemo, so I'm hoping it's on a similar pin

It's undocumented. But should JustWork

@cnlohr
Copy link
Owner

cnlohr commented Mar 1, 2025

About the debug interface, it looks like it should JustWork. The CH592 uses a QingKeV4, which has both DMDEBUG0 and DMDEBUG1 interfaces.

This is going to be a guessing game, as the datasheet does not mention the debug interfaces or a SWIO pin at all! Same for ch32x305 though, which does have a debugprintfdemo, so I'm hoping it's on a similar pin

@cnlohr how do I test this? My linkE (and linkW) are still on the slow boat, and my ESP32's are all none s2 ones 😭 edit: I just found 3 s2mini's on the bottom of my parts bin, I'm back on the horse!

There is no ESP32 programmer other than rvswd that I'm aware of and it's hard-coded to only work with the ch32v003 and not minichlink :(

@biemster
Copy link
Contributor Author

biemster commented Mar 2, 2025

There is no ESP32 programmer other than rvswd that I'm aware of and it's hard-coded to only work with the ch32v003 and not minichlink :(

so linkE is my only choice to test the debugprintfdemo? Or would the ardulink suffice?

Copy link
Owner

@cnlohr cnlohr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting pretty close. This has me pretty excited!

@biemster
Copy link
Contributor Author

Do you mind sharing notes with @dwillmore here? See what his opinions are because the 00x drive mechanism is also different than the ch32vxxx line.

yes I just quote this here to remind me

@biemster
Copy link
Contributor Author

biemster commented Mar 10, 2025

Because I'm still waiting for my linkE to test debugprintfdemo, I'm starting to forget things. Here is my summary of what I think still requires attention or a future PR:

  • test if there is really no HSI by shorting the 32M crystal (unresolved because inconclusive, I could not bother the crystal enough to stop the mcu without breaking the board)
  • see if GPIO_ModeCfg should be a function (f475b81)
  • discuss with @dwillmore what his opinions are on the gpio mechanism
  • FUNCONF_SYSTEM_CORE_CLOCK (probably just have to merge in ch59x specific stuff like CLK_SOURCE_PLL_60MHz) (4b87983)
  • test debugprintfdemo with linkE
  • new PR for flash access
  • new PR for proper interrupt handling (at least gpio, and see which peripherals need it too)
  • plenty of PRs for ADC/TEMP, PWM, SPI, I2C, BLE and whatnot

please comment if I'm missing something here?

@biemster
Copy link
Contributor Author

Now only testing debugprintfdemo is left 🎉 my linkE takes ages to arrive 😭

@biemster biemster marked this pull request as ready for review March 12, 2025 18:13
@cnlohr cnlohr merged commit ebc7b7a into cnlohr:master Mar 12, 2025
85 checks passed
@cnlohr
Copy link
Owner

cnlohr commented Mar 12, 2025

Merged per @biemster's request. A new request will be opened to address emergent issues.

@biemster biemster deleted the ch59x-bringup branch March 12, 2025 22:44
@alexandru0-dev
Copy link

@biemster sorry to ask but are there any plans for the CH58X?
they should use the same hal, the only main difference that i found is that it uses the QingKe V4A vs QingKe V4C, which supports Extended
Instruction (XW), of the 59X

@biemster
Copy link
Contributor Author

biemster commented Mar 13, 2025

@biemster sorry to ask but are there any plans for the CH58X?

I'm doing CH57X currently, which is also very similar. I don't have any ch58x, so as long that that's the case I can't work on that. (but with the ch59x and ch57x as examples it should be very straightforward to add ch58x as well, maybe you dare to do a PR @alexandru0-dev ? I can definitely help out here and there, especially if you join the discord)

@alexandru0-dev
Copy link

maybe you dare to do a PR @alexandru0-dev ?

I don't have a dev board on my hands rn, but I'm going to order one (I'm going to order the CH582F), once it arrives, I will definitely give it a try

I can definitely help out here and there, especially if you join the discord)

Can you tell me where to join?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants